Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #308 +/- ##
==========================================
- Coverage 90.69% 90.52% -0.17%
==========================================
Files 70 70
Lines 2535 2544 +9
==========================================
+ Hits 2299 2303 +4
- Misses 236 241 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| """Wait for the controller to be fully started. | ||
|
|
||
| This method blocks until the controller has completed post_initialise, | ||
| making it ready for use. |
There was a problem hiding this comment.
This does not appear to be true - it times out and returns False. I think we probably want it to propagate the exception. If it times out what is the caller supposed to do other than stop the application?
There was a problem hiding this comment.
that was not my experience - how did you test this to make that issue arise? I used it in a fastcs-catio system test and it behaved as advertised.
There was a problem hiding this comment.
I haven't tested it I am just reading the code. When called without a timeout it blocks until post_initialise is complete, but with a timeout it returns before post_initialise is complete. Is that what we want? What is the caller supposed to do if it returns False?
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
2 participants
@GDYendell as discussed yesterday. This is working as expected for fastcs-catio.
Summary by CodeRabbit
wait_for_startup()method to controllers, allowing applications to synchronize with controller startup completion. Includes optional timeout support for flexible synchronization patterns.